refactor: migrate RoomView to a function component - #7482
Open
diegolmello wants to merge 327 commits into
Open
Conversation
Add 'use memo' so React Compiler (annotation mode) returns a stable closure when inputs are unchanged, instead of a fresh arrow each call.
- hideSystemMessages returns stable refs (model field / redux prop / shared empty) instead of a fresh [] each render, so the message-list WatermelonDB query stops re-subscribing on every RoomView render - RoomView.shouldComponentUpdate ignores lastMessage for non-livechat rooms (livechat still uses it for on-hold header updates), so a new message no longer forces a full RoomView re-render
A manually revealed ignored message stayed revealed forever because the manual-reveal flag was never reset. It now resets when the isIgnored prop transitions, so the message re-hides; a benign re-render with the same value keeps the reveal. This rides on converting MessageStore to a store-only context, matching the MessageRoomStore and InteractionStore siblings: item, previousItem and isIgnored move into the zustand store state, the context carries only the store ref, field/domain/derived hooks read state inside their selectors via a private useMessageStore helper, reveal is a stable action built in the store initializer, and useMessageCtx is retired in favour of useMessageItem.
Reordering or editing quoted attachments reused the wrong Reply instance by raw index, carrying a stale loading state onto a different attachment. Key by the stable-identity expression the sibling Attachments.tsx already uses (title_link || message_link, index only as fallback).
The display msg was snapshotted from isEncrypted/tmsg into useState at mount and a []-deps effect overwrote it, so it went stale when the model updated and missed later tmid/id changes (the captured fetch closed over the initial ids). Derive the display value during render, keep state only for the fetched thread name, and depend the fetch effect on tmid/id/ displayMsg so it re-runs on change.
Status was written through scattered setStatus literals across five handlers and onPress re-branched on the status string, duplicating the transition table. Extract a downloadStatusReducer over named events (download_started/succeeded/failed/canceled, cache_hit); handlers dispatch events and onPress derives its action from state. Behavior unchanged.
- Split into a base MessageRoomStoreProvider (no useSetting) and a WithSetting variant, dispatched on whether a timeFormat prop is passed. Callers that pass timeFormat (MessagesView, SearchMessagesView) no longer subscribe to or re-render on Message_TimeFormat changes. Public MessageRoomProvider name and prop contract are unchanged. - Guard the props->store mirror effect with a shallow diff against the store's current state (keys derived from the state object, no hand-maintained key list), so setState fires only on a real change instead of every render. Reactive props still propagate.
The 233b131 rework moved the fetchThreadName call into an effect above the !tmid early return, so tmid (string | undefined) was no longer narrowed to string at the call site, breaking the type check. Guard the effect on tmid as well — the render path already returns null when tmid is absent, so fetching in that case was a no-op.
InteractionStore held interaction as two flat fields (action + selectedMessages), so invalid combos (edit with many selected ids, action out of sync with the selection) were representable and only prevented by call-site discipline. Replace them with a single discriminated union — edit/quote/react/null — and event-style action creators, making invalid states unrepresentable at the type level. Public selectors (useMessageAction, useSelectedMessages, useIsBeingEdited) are preserved by deriving from the union (useSelectedMessages via useShallow, since edit/react derive a fresh single-element array), so the composer consumers need no changes. RoomView getState() reads and createInteractionStore initializers updated to the union. ShareView drops its duplicate action/selectedMessages state; send(), onRemoveQuoteMessage and componentWillUnmount now read the interaction from the store instead of this.state.
The 233b131 change made the thread-name fetch effect re-runnable (deps went from [] to tmid/id/displayMsg/fetchThreadName). Add an ignore flag cleared on cleanup so a slow in-flight fetch can no longer overwrite the name after the inputs change.
Delivers the valid managing-state review findings as per-ticket commits: 001 Quote reply key from index to stable identity; 002 RepliedThread display derived in render + cancellable fetch; 003 media auto-download via reducer; 004 MessageRoomStore split (base vs WithSetting) + diff-guarded mirror; 005 InteractionStore discriminated union + ShareView reads the store; 006 re-hide a revealed ignored message on ignore-state transition.
…er snapshots (NATIVE-22)
…e leaves (NATIVE-22)
…n ReactionsList (NATIVE-22)
…e components (NATIVE-22)
…ved to useIsArchived (NATIVE-22)
…ts-expect-error and try/catch (NATIVE-22)
…ontext value (NATIVE-22)
…-hooks-3 # Conflicts: # CONTEXT.md # app/containers/message/hooks/useMediaAutoDownload.tsx # app/lib/methods/helpers/isReadOnly.ts
10 tasks
10 tasks
10 tasks
…ion (#7635) * test(RoomView): replace LoadMore snapshot with state assertions * fix(RoomView): cancel useThreadFollowing subscription on fast unmount useThreadFollowing subscribed inside a promise callback, so a room opened and closed before getMessageById resolved left the observer running: the cleanup ran while unsubscribe was still undefined. A cancelled flag set in cleanup and checked before subscribing closes that window, and also covers a tmid/userId change, where cleanup runs before the pending resolve. Dropped the paired suggestion to swap observe() for observeWithColumns(['replies']): observeWithColumns is a Query method, not a Model one, so it is not available on the record getMessageById returns. The derived value is a boolean, so setState already bails out when it does not change. * perf(RoomView): stop observing last_message on the room record The room observer woke on every incoming message, rebuilding roomUpdate and re-rendering the root, providers, list, footer and composer. Livechat is the only consumer of last_message: it now gets its own observer, created only for t === 'l', publishing lastMessageFromAgent into the room store. * refactor(RoomView): drop redundant hideSystemMessages re-filter * refactor(RoomView): clone the observed messages only when appending the thread record * refactor(RoomView): build the message query clauses once and drop Q.skip(0) * refactor(RoomView): skip the readThread debounce timer outside threads * refactor(RoomView): share the newer-loader lookup between the jump anchor and the rejoin * refactor(RoomView): drop the double casts to AnchorMessage * perf(RoomView): update the scroll FAB state only on a threshold crossing * refactor(RoomView): type the animated message list instead of suppressing it * refactor(message): shrink the inert message-action store to the action it serves * refactor(RoomView): flatten the jumpToMessage target checks * refactor(RoomView): share one footer action button between TakeOrJoin and OnHold * refactor(RoomView): type roomUpdate from the subscription model instead of any * test(RoomView): drop the tautological QUERY_SIZE assertion * refactor(RoomView): drop the blank-label sentinel from LeftButtons * test(RoomView): cover the visible system types clause * perf(MessageComposer): read send-time values from the store instead of subscribing * perf(RoomView): seed serverVersion into the room store at creation joinRoom no longer reads the redux singleton at call time; the screen passes the version it already selects. * perf(RoomView): skip the extra render when the acquired room store is unchanged * refactor(RoomView): resolve thread names through fetchThreadName pushThreadRoom carried its own copy of the removed-thread branch and had drifted to `Thread`; the shared helper's `Message_removed` now wins for both call paths. * refactor(RoomView): one thread-press wiring for the message tree The handlers hook wired pushThreadRoom a second time without onCancel, so the loading overlay opened there had no cancel button. It now takes the screen's onThreadPress, and onReplyInit delegates to it instead of repeating the push. * refactor(RoomView): call useReactionActions once per room screen The handlers hook built a second set of reaction actions over the same message action store; it now takes the ones the screen already created. * refactor(RoomView): one sendRoomMessage wiring per room screen The answer-button handler duplicated the screen's send wiring; it now reuses it, and the send behaviour is covered on the service itself. * refactor(message): one MessageRoomProvider The provider forked into two components only to default timeFormat; the setting is now read unconditionally and used when the caller passes none. * test(RoomView): cover pushThreadRoom and fetchThreadName pushThreadRoom is now the single owner of thread-name resolution and of both reply paths, and neither it nor the helper had a test. * feat(RoomView): render a retryable screen when room init fails useRoomInit exposes failed and retry so a room whose init exhausted its attempts no longer renders as an empty room. * fix(RoomView): cancel pending debounced calls on unmount useDebounce now returns DebouncedState so callers reach .cancel; the list onEndReached and the readThread timer no longer fire after unmount. * refactor(RoomView): share a RoomPlaceholder shell across blocking screens * refactor(RoomView): gate blocking screens before mounting the room tree RoomView's blocked-room checks ran after twelve hooks, so an invited or E2EE-blocked room opened the DDP subscription and ran init before being told it was blocked, and unblocking swapped component types at the same render position. index.tsx is now a thin RoomGate that owns the screen-identity snapshot, the room store acquisition, the header and the blocked-screen checks; the room tree moves to RoomScreen.tsx and mounts only once unblocked. RoomLoadFailed stays in RoomScreen: it needs useRoomInit's failed/retry, which belongs to the mounted room. * docs(message): remove ARCHITECTURE.md * fix(RoomView): read the Workspace version when taking an inquiry Warmed rid-keyed RoomStores were created without a server version, so joining an Omnichannel room from one selected the removed DDP path. takeInquiry now reads the version from the store at call time and the serverVersion plumbing is removed from RoomStore, joinRoom and RoomGate. * fix(RoomView): harden Jump to Message navigation and cancellation - jumping to a different Thread Parent compares the target id with the active tmid instead of relying on replies - navigation callbacks may return promises and are awaited, so failures reach the existing error handling - pushThreadRoom hides loading in a finally when the thread name lookup rejects - cancellation bumps a generation token checked after each asynchronous stage * fix(message): call the latest MessageRoomProvider callbacks Replace the frozen callback contract with stable wrappers that call the latest prop, so callbacks recreated during normal renders are honored in production without re-rendering consumers. * fix(RoomView): drop stale init failure and unreachable invite transition Failure is only exposed while the room still has initialization work, so logging out after a failed run no longer keeps the failure screen. The invite-acceptance transition effect could never run because the hook is not mounted while invited. * fix(RoomView): invalidate earlier jumps and keep store acquisition out of the state updater Each jumpToMessage call now advances the generation so an older in-flight jump cannot scroll or navigate after a newer one starts, and a stale failure no longer cancels the newer jump. useRoomStoreForScreen acquires the registry entry outside the setState updater. Adds explicit return types and an interface for RoomPlaceholderProps. * Derive room store type from subscription * fix(RoomView): cache routing config per server and derive on-hold from room state (#7638) * fix(RoomView): parse the route once into a valid screen identity or a failure state * refactor(RoomView): split RoomScreen into useRoomMessaging and focused room components RoomScreen now only wires the room lifecycle hooks and the render tree. Message orchestration (message-action store, imperative handles, navigation, init, send) lives in useRoomMessaging, grouped by the component that consumes each slice. RoomMessageList owns the message tree and its settings; RoomAnnouncementBanner, RoomUploadProgress and RoomMessageActions read the room and user themselves. useRoomSubscription creates its own RoomClass. The route-seeded quote effect in useRoomInit was a no-op (the store is already seeded with the quote) and is gone. * fix(RoomView): cache routing config per server and derive on-hold from room state (#7638) * fix(RoomView): stop the thread from opening after cancelling its loading overlay Also move the new test files into __tests__, drop the explanatory comments added in this branch, and share one style and one useTheme call in RoomPlaceholder. * refactor(RoomView): pass RoomGate props explicitly and drop the route parser comment * chore(RoomView): drop references to the removed ARCHITECTURE.md * fix(RoomView): replace the store room when the subscription row is recreated observeRoom only rewrote room when a roomAttrsUpdate attribute differed from the previous snapshot, so a subscription row recreated with identical attributes (leave and rejoin while the screen is open) left the store pointing at the deleted model instance. A lastMessage-only change never had this problem: WatermelonDB re-emits the same cached instance, so the stored room already reflects it. * fix(RoomView): clear livechat-only flags when the room type is not livechat lastMessageFromAgent was only recomputed while the subscription row had t === 'l', and useOmnichannelPermissions returned early for any other type, so both left their last livechat value in the store. Derive every flag unconditionally: non-livechat rows always write false. * fix(RoomView): reset the cached routing config on logout Since 5c7a16c the omnichannel routing config is cached per server URL in a module-level store. Logging out and back into the same URL kept the cached returnQueue value, so an admin-side routing change never reached the client. The store now resets when logout runs, matching the per-mount refetch that existed before the cache. * fix(RoomView): gate setParams thread jumps on loaded thread messages A jumpToMessageId delivered via setParams to a thread RoomView fired immediately, bypassing the onThreadMessagesLoaded gate the mount path uses. Before the thread window is populated a non-anchored thread jump aborts and parks on the live tail. The setParams path now parks the id in the same pending slot until the thread messages have loaded. * refactor(RoomView): reuse the livechat check and rename the non-livechat flags test * refactor(RoomView): name the recreated-row check in observeRoom * fix(RoomView): key the thread jump gate to the loaded thread and tighten its test * fix(RoomView): reset the routing config cache on account deletion and cover logout in tests * test(RoomView): cover the thread jump gate across a thread switch * refactor(logout): reset the routing config cache in removeServerData * test(logout): drop the duplicated routing config reset case * refactor(RoomView): pass useRoomMessaging results as explicit props Flatten the hook result so RoomScreen names every prop it passes instead of spreading consumer-shaped bundles, drop the explanatory comments, and keep room init ahead of the room subscription as before the split. * refactor(RoomView): useRoomMessaging has no JSX, use .ts * fix(RoomView): re-render the banner and message list on room updates * test(RoomView): replace adapter-dependent visible-row tests with predicate unit tests Also moves visibleSystemMessages out of the hooks folder, since it exports no hooks. * Move routing config cache to omnichannel redux
This was referenced Sep 3, 2026
Pass provider props explicitly instead of spreading, flatten MessageRow into early returns, route goSearchView through navigateToScreen so the ts-ignore can go, and read settings through useSetting. Hoist RoomPlaceholder's stylesheet to module scope, drop Banner's memo comparator that ignored title and closeBanner, and remove InvitedRoom's unused loading prop.
* refactor(MessageComposer): restore memoization and narrow store subscriptions ComposerInput emits React Compiler errors, so it is never auto-memoized: restore its memo wrapper and the useFocusEffect useCallback deps, which the emitter cleanup depends on. Hoist MessageComposerContainer's default children out of JSX so the file compiles again, narrow CancelEdit and useChooseMedia to useMessageActionKind, name the a11y announce delay and use optional call syntax for onClosed. * chore: keep List hook tests in place Two test file moves were committed here by mistake; they belong with the hooks changes that fix their import paths.
10 tasks
…rection (#7643) Collapse multi-field room store reads into single useShallow selectors, rebuild useUnreadsCount on the shared useObservable helper, remove a duplicate rid subscription and a redundant live-ref layer, give the navigation mirror effect a dependency array, pass RoomHeader props explicitly, and move the two stray List hook tests into __tests__.
10 tasks
* refactor(RoomView): own room stores per screen * refactor(RoomView): tidy room store and services Return a discriminated result from loadRoom, dedupe init, and cut observeRoom down to a single state read with the room snapshot built only when the room actually changes. Derive blockAction params from the trigger type, split pushThreadRoom's name accumulator, name the jump commit wait, and move the store and service tests into __tests__. * refactor(RoomView): re-apply the room store registry removal lost in a merge Merge 0cf4e58 revived the rid-keyed registry that 201e293 removed. It had no production callers, so this deletes it again on top of the tidied store, collapses the observeRoom overload it required, threads init's abort signal into loadRoom, and drops the tests that only covered registry lifetime. * refactor(RoomView): move omnichannel tests into __tests__ and drop stale rid-keyed claims The routing-config suites were colocated instead of living under __tests__, and its reducer imported actionsTypes twice. The room/thread screen suite and useSubscriptionUnreads still described a rid-keyed store that no longer exists. * test(MessageRoomStore): pass reactionInit explicitly instead of spreading a partial state
|
iOS Build Available Rocket.Chat 4.77.0.109576 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Migrates
app/views/RoomViewfrom a 1726-line class component to a function component. Behavior-preserving — no user-visible change intended.shouldComponentUpdate;connect(mapStateToProps)and all HOCs preserved.RoomStoreregistry (stores/RoomStore.ts): self-hydrates from the DB, shared by a room and its threads, torn down on last release;goRoomwarms it at nav time.RoomContextreplaced by a per-instance composer Zustand store (stores/ComposerStore.tsx).useRoomInit,useRoomAudioLifecycle,useRoomRemoved,useHeader,useJumpToMessage,useMessageActions,useOmnichannelPermissions,useRoomNavigation) and presentational components (MessageRow,RoomFooter,RoomMessageActions, room-state screens).'use memo'throughout so the React Compiler owns memoization.SearchMessagesViewresults are wrapped inA11yGateProviderso long-press message actions work there too — a small a11y addition riding along with the shared message-handler extraction.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-34
How to test or reproduce
Use a room end to end (send/edit/quote/react, drafts, autocomplete, threads, jump-to-message, join a not-subscribed room, header actions) on phone and tablet, plus an omnichannel/livechat room. Behavior should match the base branch.
Screenshots
Types of changes
Checklist
Further comments
Stacked on
native-22-message-hooks(PR #7455) and targets it until NATIVE-22 lands ondevelop, after which this will be rebased and retargeted todevelop.Summary by CodeRabbit